Fix any()/all() reductions with masked subsets and scalar semantics - #92
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31b94e9d2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (identical(call_name, "any")) { | ||
| glue("(({x}) .and. ({hoisted_mask}))") | ||
| } else { | ||
| glue("((.not. ({hoisted_mask})) .or. ({x}))") |
There was a problem hiding this comment.
Treat length-1 mask constructors as scalars in reductions
When a logical mask like c(TRUE) is used in subsetting (e.g., any(x[c(TRUE)]) with x a length>1 vector), [ hoists the mask as a rank-1 array constructor. This branch then emits any((x) .and. ([.true.])) or all((.not. ([.true.])) .or. (x)). In Fortran, a length-1 array constructor is not conformable with a length-N array, so the .and./.or. operations are invalid and the generated code will fail to compile. The mask needs to be treated as a scalar in this case (e.g., by reducing it with any() or special-casing length-1 constructors) to match R’s recycling semantics.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
==========================================
+ Coverage 92.76% 92.78% +0.01%
==========================================
Files 27 27
Lines 5734 5749 +15
==========================================
+ Hits 5319 5334 +15
Misses 415 415 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Tests: